Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Critical defects can block card registration, permanently defer probing, overlap companion-stream payload slots, and omit codec prefixes required to avoid control-name collisions and resolve routes.
5 open issues
Reserve payload space for companion peripheral ports · New Reset payload offsets after allocating all peripheral ports · New Add companion-only codecs to codec configuration mappings · New Set a valid platform component name for companion links · New Defer ordinary endpoint lookup until after presence checks · New
What changed in this PR
This PR adds shared ASoC and SoundWire support for companion amplifiers that receive audio from another peripheral rather than the host.
Changes:
- Adds companion endpoint metadata, grouping, and RT712/RT1320 DAI entries.
- Creates Intel codec-to-codec links and adapts AMD drivers to the updated endpoint helpers.
- Introduces a companion stream type and adjusts peripheral port offsets.
| File | Description |
|---|---|
sound/soc/sof/intel/hda.c |
Excludes companion endpoints from automatic discovery. |
sound/soc/sdw_utils/soc_sdw_utils.c |
Adds companion DAI entries and endpoint grouping. |
sound/soc/intel/boards/sof_sdw.c |
Creates companion codec-to-codec links. |
sound/soc/amd/acp/acp-sdw-sof-mach.c |
Adapts SOF endpoint parsing for companion groups. |
sound/soc/amd/acp/acp-sdw-legacy-mach.c |
Adapts legacy endpoint parsing for companion groups. |
include/sound/soc-acpi.h |
Adds the companion endpoint flag. |
include/sound/soc_sdw_utils.h |
Defines companion grouping structures and helper interfaces. |
include/linux/soundwire/sdw.h |
Adds the companion stream type. |
drivers/soundwire/generic_bandwidth_allocation.c |
Resets port offsets for companion streams. |
Review effort: Balanced
plbossart
left a comment
There was a problem hiding this comment.
Man you seem to start really big SoundWire changes. You should hire me haha...
Overall I didn't quite get how the new dailink is supposed to be handled, specifically how it's allocated, prepared and triggered. It's quite dependent on the host-peripheral dailink.
| num_links = sdw_be_num + ssp_num + dmic_num + hdmi_num + bt_num + 1; | ||
| dai_links = devm_kcalloc(dev, num_links, sizeof(*dai_links), GFP_KERNEL); | ||
| num_links = sdw_be_num + ssp_num + dmic_num + hdmi_num + bt_num + num_comp_amps + 1; | ||
| dai_links = devm_kcalloc(dev, num_links + 1, sizeof(*dai_links), GFP_KERNEL); |
There was a problem hiding this comment.
you should explain why there's an additional dailink.
What's not clear specifically is how this dailink is handled, i.e. how it's allocated, prepared, triggered.
so far the device-to-device connections are not handled by the SoundWire core, so it's not clear how this might work?
There was a problem hiding this comment.
That is a codec to codec dai link to connect 2 peripherals. I will add some commits from @shumingfan to this PR to show how the link might work.
| SDW_STREAM_PCM = 0, | ||
| SDW_STREAM_PDM = 1, | ||
| SDW_STREAM_BPT = 2, | ||
| SDW_STREAM_COMPANION = 3, |
There was a problem hiding this comment.
yeah but it could be PDM or PCM.
It think you'd need to add a new transport type, such as MANAGER_PERIPHERAL or PERIPHERAL_PERIPHERAL.
There are other uses cases where device-to-device communication is needed, e.g. to share the amp feedback. Don't make this just for the companion case.
| slave_total_ch += ch; | ||
|
|
||
| if (m_rt->stream->type == SDW_STREAM_COMPANION) | ||
| port_bo = t_data->block_offset; |
There was a problem hiding this comment.
This is trying to let all peripherals use the same frame slots. Like rt1320 and rt712 use the same frame slots.
| * the mach table if it is present. | ||
| */ | ||
| if (!is_endpoint_present(sdw_device, codec_info_list[i].dais[j].dai_type) || | ||
| (codec_info_list[i].dais[j].quirk & SOC_SDW_COMPANION_AMPS)) |
There was a problem hiding this comment.
the commit message says the opposite of what's done in other patches where you create a new dailink.
There was a problem hiding this comment.
Trying to skip the companion dais here to avoid creating the dai links that connect the companion dai to the host. And the companion link will be created later by create_sdw_comp_dailinks().
Add asoc_sdw_companion_amp_endpoint struct to help machine driver to create codec to codec dai links for SoundWire companion amps. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
The sdw_stream_config structures in the manager drivers are not initialized and only some of the fields are set. Initialize them to zero so that any field added to the structure later, and not explicitly set by these drivers, gets its default value instead of stack garbage. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
The companion stream is from peripheral to peripheral and all peripherals need to use the same SoundWire frame area to transfer the stream data. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
The companion amp dai links should not be created by the default machine driver. We will create a mach table for the config that companion amp is used. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Shuming Fan <shumingf@realtek.com>
Signed-off-by: Shuming Fan <shumingf@realtek.com>
Signed-off-by: Shuming Fan <shumingf@realtek.com>
Add the componion amp dai info for rt712 and rt1320. This commit is only for testing which is not a correct information. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
e890ae0 to
ced34fe
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Undefined DAI and stream constants prevent compilation, while a missing Kconfig dependency and critical transport, port-allocation, and teardown defects create further build and runtime risks.
8 open issues
Define the missing companion DAI type · New Add the required sdw-utils Kconfig dependency · New Use the defined peripheral transport type · New Initialize the stream configuration transport · New Keep the bookkeeping port out of hardware programming · New Clean up streams after preparation or enable failures · New Avoid changing microphone clocks when configuring AIF3 · New Release stream runtimes during shutdown · New
5 resolved since last review
Review effort: Balanced
| * the SoundWire bus instead of from the host, so a codec-to-codec dai link is created between | ||
| * the main amp and the companion amps. | ||
| */ | ||
| #define SOC_SDW_COMPANION_AMPS BIT(18) |
| .probe = asoc_sdw_c_amp_probe, | ||
| .startup = asoc_sdw_c_amp_startup, | ||
| .prepare = asoc_sdw_c_amp_prepare, |
| return PTR_ERR(sdw_stream); | ||
| } | ||
|
|
||
| sdw_stream->type = SDW_STREAM_COMPANION; |
| { | ||
| struct sdw_slave *slave = dev_to_sdw_dev(dai->dev); | ||
| struct sdw_stream_runtime *sdw_stream = NULL; | ||
| struct sdw_stream_config sconfig; |
| pconfig.num = 0; // no meaning for companion stream | ||
|
|
||
| retval = sdw_stream_add_master(slave->bus, &sconfig, &pconfig, 1, sdw_stream); |
| ret = sdw_disable_stream(sdw_stream); | ||
| if (ret) { | ||
| dev_err(dai->dev, "%s trigger (disable stream) failed: %d\n", __func__, ret); | ||
| return ret; | ||
| } | ||
|
|
||
| ret = sdw_deprepare_stream(sdw_stream); | ||
| if (ret) { | ||
| dev_err(dai->dev, "%s deprepare failed: %d\n", __func__, ret); | ||
| return ret; | ||
| } |
| retval = sdw_stream_add_slave(rt1320->sdw_slave, &stream_config, | ||
| &companion_config, 1, sdw_stream); |
| snd_soc_dai_stream_dma_data_set_playback(dai, NULL); | ||
| snd_soc_dai_stream_dma_data_set_capture(dai, NULL); |


Some platforms use a SoundWire companion amp, which gets its audio data
from another SoundWire peripheral (the main amp) instead of from the
host. The current machine drivers and SoundWire core assume that every
stream is exchanged with the host, so these amps cannot be described or
configured.
This series adds the common support needed for such configurations:
a way to mark companion amp endpoints in the mach table, machine driver
support for the peripheral to peripheral path, a SoundWire stream type
that fits it.